Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

AP-1335 Fix MenuItems not being truncated properly #158

Merged
merged 4 commits into from
Feb 14, 2020

Conversation

justinanastos
Copy link
Contributor

This was the result of a lot of tinkering. This makes it possible to truncate menu items 🤷‍♂

@justinanastos justinanastos changed the title Fix MenuItems not being truncated properly AP-1335 Fix MenuItems not being truncated properly Feb 13, 2020
@justinanastos justinanastos force-pushed the justin/menus/AP-1335-help-truncation branch from 6c6fbf9 to 2e3fe79 Compare February 13, 2020 21:40
Copy link
Contributor

@jgzuke jgzuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird, maybe a story so it doesnt break later without anyone noticing

@justinanastos
Copy link
Contributor Author

@jgzuke

Weird, maybe a story so it doesnt break later without anyone noticing

Thanks for calling me out; I was being lazy. Story added!

"forced visible",
() => {
return (
<PerformUserInteraction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, will these always trigger the new state before the snapshot is taken? I like this a lot better than testing only props. Would love to use this pattern in agm as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be incorporated into AGM in https://github.com/mdg-private/engine-frontend/pull/2016.

While I think this is a step in the right direction, it is not an ideal solution still. Chromatic does not have an async rendering feature (https://docs.chromaticqa.com/resource-loading#asynchronous-rendering), so we have to manually add { chromatic: { delay: number } } as the second argument to .add. @timbotnik suggested I write a blog post on this and gently nudge the chromatic team to offer the ability to programmatically delay snapshots until something has happened, which I think we be awesome.

All that being said, I'm ok adding a 2 second delay to a chromatic capture since they all run in parallel and this would enable us to write less magic props to force component states.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To answer your question more succinctly

will these always trigger the new state before the snapshot is taken

No, it will not. We have to manually delay the snapshot with a best-guess time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, that makes more sense. I still really like this setup though! Wasnt a huge fan of test props for stories

@justinanastos justinanastos merged commit 0d8f1d6 into master Feb 14, 2020
@justinanastos justinanastos deleted the justin/menus/AP-1335-help-truncation branch February 14, 2020 14:34
@apollo-bot2
Copy link
Collaborator

🚀 PR was released in v3.0.1 🚀

@apollo-bot2 apollo-bot2 added the released This issue/pull request has been released. label Feb 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants